Use masked input for Backpex.Fields.Currency
#1307
Conversation
Flo0807
force-pushed
the
feature/1280-improve-currency-field
branch
from
05f2d56 to
1f67ccd
Compare
Flo0807
force-pushed
the
feature/1280-improve-currency-field
branch
2 times, most recently
from
1f67ccd to
c212eae
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR improves the Backpex.Fields.Currency field by replacing the internal Money library implementation with a more flexible masked number input approach. The changes allow users to provide their own Money library configuration while maintaining proper currency formatting in forms.
Key changes:
- Removes the internal
Backpex.Ecto.Amount.Typeand switches to external Money library usage - Introduces a new masked number input component with IMask.js integration
- Adds configurable currency formatting options (unit, position, separators)
Reviewed Changes
Copilot reviewed 12 out of 18 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| lib/backpex/fields/currency.ex | Updated to use masked input instead of number input, added configuration options for currency formatting |
| lib/backpex/html/form.ex | Added new masked_number_input component with IMask integration |
| assets/js/hooks/_masked_number_input.js | New JavaScript hook implementing IMask for currency formatting |
| lib/backpex/ecto/amount_type.ex | Removed internal Money type implementation |
| test/ecto/amount_type_test.exs | Removed tests for the deleted Amount type |
| package.json | Added imask dependency |
| mix.exs | Removed money dependency from main project |
| demo/ files | Updated demo to use external Money library with proper configuration |
| end | ||
|
|
||
| @impl Backpex.Field | ||
| def search_condition(schema_name, field_name, search_string) do |
There was a problem hiding this comment.
Still, the UX when searching for currency is not very good, as you have to search for integer values. We could add another field_option that is used in this function to convert the search string into an integer. (However, we would need to modify the function to receive the field options...)
WDYT? @krns @pehbehbeh
There was a problem hiding this comment.
Does the same apply for Filters like a Range filter?
To have some kind of options to transform the search term sounds good.
There was a problem hiding this comment.
We have the same problem when using a range filter. The search_condition is not used there, so I guess we should leave it like it is and find a solution that covers both filters and field search.
Co-authored-by: Simon Hansen <[email protected]>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Flo0807
force-pushed
the
feature/1280-improve-currency-field
branch
from
247debd to
1c81961
Compare
Yes, it works. |
Flo0807
changed the title
Backpex.Fields.CurrencyBackpex.Fields.Currency
Co-authored-by: Phil-Bastian Berndt <[email protected]>
| price: %{ | ||
| module: Backpex.Fields.Currency, | ||
| label: "Price", | ||
| unit: "€", | ||
| unit_position: :after, | ||
| radix: ",", | ||
| thousands_separator: "." | ||
| }, | ||
| ... | ||
| ] | ||
| end | ||
| ``` | ||
|
|
||
| See [field-specific options](`Backpex.Fields.Currency`) for default values. | ||
|
|
||
| 2. Configure defaults for `money` in your `config/config.exs` | ||
|
|
||
| ```elixir | ||
| config :money, | ||
| default_currency: :EUR, | ||
| separator: ".", | ||
| delimiter: ",", | ||
| symbol_on_right: true, | ||
| symbol_space: true |
There was a problem hiding this comment.
Align config keys:
- separator vs. thousands_separator
- delimiter vs. radix
- symbol_on_right vs. unit_position
- symbol_space not available on field level?
There was a problem hiding this comment.
Ahh, got it.. thats the money config vs. backpex config.. hmhmh
closes #1280